-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[PowerPC] fix float ABI selection on ppcle #154773
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang-codegen Author: None (DanilaZhebryakov) Changessoft float ABI selection was not taking effect on little-endian powerPC with embedded vectors (e.g. e500v2) leading to errors. Full diff: https://github.com/llvm/llvm-project/pull/154773.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 677d8bc82cb0a..8b85b93b5d568 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -188,7 +188,8 @@ createTargetCodeGenInfo(CodeGenModule &CGM) {
return createPPC32TargetCodeGenInfo(CGM, IsSoftFloat);
}
case llvm::Triple::ppcle: {
- bool IsSoftFloat = CodeGenOpts.FloatABI == "soft";
+ bool IsSoftFloat =
+ CodeGenOpts.FloatABI == "soft" || Target.hasFeature("spe");
return createPPC32TargetCodeGenInfo(CGM, IsSoftFloat);
}
case llvm::Triple::ppc64:
|
@llvm/pr-subscribers-clang Author: None (DanilaZhebryakov) Changessoft float ABI selection was not taking effect on little-endian powerPC with embedded vectors (e.g. e500v2) leading to errors. Full diff: https://github.com/llvm/llvm-project/pull/154773.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 677d8bc82cb0a..8b85b93b5d568 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -188,7 +188,8 @@ createTargetCodeGenInfo(CodeGenModule &CGM) {
return createPPC32TargetCodeGenInfo(CGM, IsSoftFloat);
}
case llvm::Triple::ppcle: {
- bool IsSoftFloat = CodeGenOpts.FloatABI == "soft";
+ bool IsSoftFloat =
+ CodeGenOpts.FloatABI == "soft" || Target.hasFeature("spe");
return createPPC32TargetCodeGenInfo(CGM, IsSoftFloat);
}
case llvm::Triple::ppc64:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a regression test.
I'm not sure the Target.hasFeature("spe")
is really what we want to be doing here. It looks like spe is actually a mutually incompatible ABI, not just a CPU feature, and we should model it as such. But it matches big-endian ppc, so I won't block this patch on fixing that.
96fde1d
to
8e05a06
Compare
Added test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you unify the test with clang/test/CodeGen/PowerPC/ppc-sfvarargs.c, since it's testing basically the same thing?
3995f52
to
542f85b
Compare
Done. It appears that existing checks in clang/test/CodeGen/PowerPC/ppc-sfvarargs.c are sufficient to check that correct ABI is used. |
33ac551
to
8ad5395
Compare
also oops, some weird changes are shown due to accidental merge rather than rebase. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
We have automation that's supposed to trigger, but since it isn't:
|
Done. If everything is ok, please merge it. |
soft float ABI selection was not taking effect on little-endian powerPC with embedded vectors (e.g. e500v2) leading to errors.
(embedded vectors use "extended" GPRs to store floating-point values, and this caused issues with variadic arguments assuming dedicated floating-point registers with hard-float ABI)